test(frost): real-cgo interactive signing e2e via a DKG-persisted keyGroup (RunDKG)#4097
Open
mswilkison wants to merge 3 commits into
Open
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…Group (RunDKG) Completes the real-cgo INTERACTIVE signing end-to-end test: a full FROST DKG that PERSISTS a key group (RunDKG), followed by an interactive ROAST signing round driven through the engine's interactive session API (DeriveInteractiveAttemptContext -> InteractiveSessionOpen -> InteractiveRound1 -> NewSigningPackage -> InteractiveRound2 -> InteractiveAggregate) with real frost-secp256k1-tr crypto. It fills the gap in real-cgo coverage: - _WithLinkedSigner proves the real crypto over the LOW-LEVEL Sign/Aggregate API (fed KeyPackage bytes directly); - the fake-engine runner suite proves the Go-side orchestration without crypto. This is the missing combination - the interactive session API the runner uses, with real crypto - the prerequisite toward coarse-path retirement. THE keyGroup GLUE = RunDKG. InteractiveSessionOpen resolves the signing key by a keyGroup IDENTIFIER (engine-internal persisted material), not KeyPackage bytes, so the low-level DKG result the test holds as bytes is not loadable as an interactive keyGroup. RunDKG runs the full DKG, persists the result, and returns the keyGroup the interactive (and coarse) signing path resolves - the same flow production uses (the wallet runs DKG, gets a keyGroup, then signs). Correctness is proven by the engine's SUCCESSFUL InteractiveAggregate: FROST validates the shares and the aggregate internally, so a non-error 64-byte BIP-340 signature is a valid threshold signature over the message under the keyGroup's group key. The one remaining nicety, an ADDITIONAL external schnorr.Verify, is intentionally omitted: the engine exposes no keyGroup -> group-pubkey accessor (RunDKG returns only the keyGroup id), so external verification would need a new engine API. The e2e itself is complete via the internal validation. Skip-guarded for absent linked tbtc-signer FFI symbols, so it is inert in CI builds that do not link the Rust signer and runs only where the lib is present. Verifiable in this env (no lib): cgo build/vet + gofmt clean and a clean SKIP; the DKG and interactive round run only where the lib is linked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3debdc3 to
67b9530
Compare
…old) Codex review fold on #4097. RunDKG persists the DKG result in the signer's encrypted state, so a LINKED signer environment needs a state encryption key and an isolated state path - which the existing _WithLinkedSigner test did not need because its low-level Part1/2/3 DKG does not persist. Without them this e2e fails before signing (TBTC_SIGNER_STATE_ENCRYPTION_KEY_HEX missing), and a shared/default state path plus the fixed session id makes reruns conflict. Set both before RunDKG: a deterministic 32-byte test encryption key, and a fresh TBTC_SIGNER_STATE_PATH under t.TempDir() so each run starts from clean state. This makes the test self-contained in a linked environment (no external state setup, rerunnable). Still skips cleanly when the FFI symbols are absent. Verified: cgo build/vet + gofmt clean; the test still runs and SKIPS cleanly without the Rust lib. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… just RunDKG Validated against a LINKED libfrost_tbtc: the e2e gets through RunDKG (the keyGroup glue and the state isolation both work against the real lib) but a dylib predating frost_tbtc_derive_interactive_attempt_context made DeriveInteractiveAttemptContext return "unavailable", which the test treated as a hard failure - so a present-but- stale lib failed instead of skipping. Generalize the skip guard: skipFrostUnavailable turns ErrNativeCryptographyUnavailable from ANY engine call into a SKIP that names the operation and says to rebuild the lib, while any other error stays a real failure. Now the e2e is inert when the lib is absent OR incomplete, and runs to completion only against a current lib. Also document the link-and-run invocation in the file header. Verified: cgo build/vet + gofmt clean; skips without the lib (at RunDKG) and, against the stale lib, skips cleanly at the derive step (was failing) naming the missing symbol. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The real-cgo interactive signing end-to-end test, now with the keyGroup glue wired: a full FROST DKG that persists a key group (
RunDKG), followed by an interactive ROAST signing round driven through the engine's interactive session API (DeriveInteractiveAttemptContext→InteractiveSessionOpen→InteractiveRound1→NewSigningPackage→InteractiveRound2→InteractiveAggregate) with realfrost-secp256k1-trcrypto.It fills the gap in real-cgo coverage:
_WithLinkedSignerproves the real crypto over the low-levelSign/AggregateAPI (fedKeyPackagebytes directly);This is the missing combination — the interactive session API the runner uses, with real crypto — the prerequisite toward coarse-path retirement.
The keyGroup glue =
RunDKGInteractiveSessionOpenresolves the signing key by a keyGroup identifier (engine-internal persisted material), notKeyPackagebytes — so a low-level DKG result the test holds as bytes isn't loadable as an interactive keyGroup.RunDKGruns the full DKG, persists the result, and returns the keyGroup the interactive (and coarse) signing path resolves — the same flow production uses (the wallet runs DKG → keyGroup → signs).Correctness & the one documented limitation
Correctness is proven by the engine's successful
InteractiveAggregate: FROST validates the shares and the aggregate internally, so a non-error 64-byte BIP-340 signature is a valid threshold signature under the keyGroup's group key. An additional externalschnorr.Verifyis intentionally omitted: the engine exposes no keyGroup → group-pubkey accessor (RunDKGreturns only the keyGroup id, andExtractDkgGroupPublicKeyFromMaterialneeds material the test doesn't hold), so external verification would need a new engine API. The e2e is complete via the internal validation; that accessor is the one remaining nicety.Scope / verification
Test-only,
frost_native && frost_tbtc_signer && cgotag. Skip-guarded for the absence of the linked tbtc-signer FFI symbols, so it's inert in CI builds that don't link the Rust signer and runs only where the lib is present.Verifiable here (no Rust lib): cgo build/vet + gofmt clean, and the test runs and skips cleanly. The DKG and interactive round themselves run only where the lib is linked — this PR wires the full flow and the keyGroup glue.
🤖 Generated with Claude Code